Ignore new rails rate limit errors #2774
Conversation
solnic
left a comment
solnic
left a comment
There was a problem hiding this comment.
Thanks for the PR! Please add tests too and we could merge this in 😄
| "ActionController::NotImplemented", | ||
| "ActionController::ParameterMissing", | ||
| "ActionController::RoutingError", | ||
| "ActionController::TooManyRequests", |
There was a problem hiding this comment.
Please add it conditionally if it's rails >= 8.0
Add 'ActionController::TooManyRequests' to error list
camallen
force-pushed
the
ignore-new-rails-rate-limit-errors
branch
from
40128cc to
f4804e7
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| after(:initialize) do | ||
| @rails = Sentry::Rails::Configuration.new | ||
| @excluded_exceptions = @excluded_exceptions.concat(Sentry::Rails::IGNORE_DEFAULT) | ||
| @excluded_exceptions = @excluded_exceptions.concat(Sentry::Rails::RAILS_8_IGNORE_DEFAULT) if ::Rails.version.to_f >= 8.0 |
There was a problem hiding this comment.
Bug: Rate limit check uses wrong Rails version
The code checks for Rails 8.0 to ignore ActionController::TooManyRequests, but rate limiting was actually introduced in Rails 7.2. This means Rails 7.2 and 7.x users won't have rate limit errors ignored by Sentry, even though they have the feature. The version check, comment, and constant name all incorrectly reference Rails 8.0 instead of 7.2.
Additional Locations (2)
There was a problem hiding this comment.
Good point - however before rails 8.1.1 it used to respond with a head :too_many_requests response, not raise this new error we're ignoring, see rails/rails@73ecd0c for details.
Add Rails 8.1.1 rate limiting error to excluded exceptions conditionally. Only applies when Rails version >= 8.1.1.
camallen
force-pushed
the
ignore-new-rails-rate-limit-errors
branch
from
f4804e7 to
775d6f9
Compare
2 participants
Add rails 8 rate_limit activation error